🐛 Fix preserving case in enum values #571
Conversation
|
📝 Docs preview for commit a933d91 at: https://64199c483889490814ce2f68--typertiangolo.netlify.app |
|
📝 Docs preview for commit 43a56b8 at: https://6419a27ba36fd00c80fd7b0a--typertiangolo.netlify.app |
|
📝 Docs preview for commit a0061c9 at: https://649016e778649d3efba5baf1--typertiangolo.netlify.app |
|
Is there any reason why this has yet to be merged? Can I help? |
Is there something I should do? |
There was a problem hiding this comment.
Hi, thanks for the fix!
I could replicate the bug on master and verify that this PR fixes the issue. I looked into the code edits in detail and those look fine to me as well.
I do think that the original example from #570 was slightly more clear, so I'll go ahead and update the test case you've included if that's OK.
|
📝 Docs preview for commit 40e2e92 at: https://7cb850dc.typertiangolo.pages.dev |
|
📝 Docs preview for commit 47520b3 at: https://ec1ef388.typertiangolo.pages.dev |
There was a problem hiding this comment.
This looks good to me, though I'd love a quick verification by @tiangolo on the following points:
- The original code called
.lower()clearly on purpose - are there any use-cases we're not supporting anymore after this rewrite? I don't personally think so as users can set thecase_sensitiveflag ontyper.Option(as documented). Still, should we consider this PR as potentially breaking user's previous code? Which version of the test would you like to keep? There'stests/test_enum_case.pybut I also included a version withdocs_src/parameter_types/enum/tutorial003.py. If we want the latter, we can go ahead and deletetests/test_enum_case.py.
|
📝 Docs preview for commit 0efe55c at: https://583ed24c.typertiangolo.pages.dev |
|
📝 Docs preview for commit caecbd2 at: https://68497888.typertiangolo.pages.dev |
|
After internal discussion with @tiangolo, I updated the PR further to use the same example from the docs, and to add a note to the documentation. The current documentation has a specific section on how to make the enums case insensitive, which seems to confirm that the current behaviour on |
|
📝 Docs preview for commit 53a94e4 at: https://8461de1d.typertiangolo.pages.dev |
|
Awesome, thanks @avaldebe! 🚀 And thanks @krzysztofwos for the report. 🤓 Also, thanks @svlandeg for the help! 🙇 This will be available in Typer 0.9.1 in the next few hours. 🚀 |
fixes #570: Enum values that differ in case get conflated